feat(signal): add typed legacy session interop - #1072

wacore/libsignal/src/protocol/session_cipher.rs (1)

2130-2133: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Round-trip the record through persistence in this regression.

This rebuilds an in-memory SessionRecord, but never serializes and reloads it. Add a serialization/reload round-trip before decrypting first to verify that the archived skipped key survives actual persisted state.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wacore/libsignal/src/protocol/session_cipher.rs` around lines 2130 - 2133,
Update the regression setup around bob_sessions and
SessionRecord::from_components to serialize the rebuilt record through the
existing persistence mechanism, reload it, and use the reloaded record before
decrypting first. Preserve the test’s existing session address and decryption
flow while ensuring the archived skipped key is validated from persisted state.

wacore/libsignal/src/protocol/legacy_session.rs (1)

415-416: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

This error text lies about what actually went wrong. Fix it before it ships.

InvalidSignedPreKeyId says "is out of range," but per the library-context notes, out-of-range validation for the signed pre-key id was removed. The only remaining trigger (line 1137-1140) is a missing signed_pre_key_id (None), not an out-of-range value. Ship a message that actually describes the failure — I don't want engineers chasing ghosts in the logs.

🩹 Proposed fix

-    #[error("legacy session {session} signed pre-key id is out of range")]
+    #[error("legacy session {session} is missing a signed pre-key id")]
     InvalidSignedPreKeyId { session: usize },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wacore/libsignal/src/protocol/legacy_session.rs` around lines 415 - 416,
Update the error message for InvalidSignedPreKeyId to describe a missing signed
pre-key id rather than an out-of-range value. Keep the error variant and its
session field unchanged, and align the text with the None-triggered failure at
the existing call site.

wacore/derive/src/lib.rs (1)

1117-1226: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Make ConnectFailureReason explicit about missing fallback resilience

VideoState and TempBanReason still have #[wire_fallback], which is appropriate. ConnectFailureReason omits it, but that may intentionally hard-fail on unknown server codes before logging out. Make that contract explicit in the enum docs so a future refactoring does not accidentally re-add forward-compatibility where strict failure was meant.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wacore/derive/src/lib.rs` around lines 1117 - 1226, Update the documentation
for ConnectFailureReason to explicitly state that it intentionally omits
#[wire_fallback] and strictly rejects unknown server codes. Preserve the
existing strict conversion behavior and do not add fallback handling; clarify
that this hard failure is intentional before logout.

wacore/libsignal/src/protocol/session_cipher.rs (1)

1361-1392: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Closed branch logic is sound — but where's the test for the counter >= next_index rejection?

The bounds check at Lines 1373-1377 is a brand-new failure mode (InvalidSessionStructure("receiver chain is closed")) for a message that's genuinely beyond recovery once the chain key bytes are gone. None of the three new regression tests exercise it — they all stay under next_index. We don't ship security-critical ratchet code without a test for every new branch; add a case where counter is at or past next_index on a closed chain and assert the error.

🧪 Sketch of the missing test

#[test]
fn closed_receiver_chain_rejects_a_counter_at_or_past_next_index() {
    let (mut tp, first, mut components) = setup_skipped_key_scenario();
    let mut rng = rand::make_rng::<rand::rngs::StdRng>();

    let current = components.current_session.as_mut().expect("current");
    let receiver = receiver_chain_for(current, &first);
    receiver.chain_key.as_mut().expect("chain key").key = None;
    // Leave next_index as-is, but drop the skipped key for `first` so the
    // only remaining candidate for a counter >= next_index is rejected.
    receiver.message_keys.retain(|k| k.index != Some(first.counter()));
    install_record(&mut tp, components);

    futures::executor::block_on(async {
        let err = message_decrypt_signal(
            &first,
            &tp.alice_addr,
            &mut tp.bob_sessions,
            &mut tp.bob_identity,
            &mut rng,
        )
        .await
        .expect_err("counter beyond a closed chain's next_index must fail");
        assert!(matches!(err, SignalProtocolError::InvalidMessage(_, _)));
    });
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wacore/libsignal/src/protocol/session_cipher.rs` around lines 1361 - 1392,
Add a regression test alongside the existing session-cipher tests for
`decrypt_with_pending_state`’s closed receiver-chain branch, using the
skipped-key setup to clear the chain key, remove the relevant message key, and
submit a counter equal to or greater than `next_index`; assert decryption fails
with the expected invalid-message-wrapped error while preserving the existing
tests.

wacore/derive/src/lib.rs (1)

1162-1199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Good, the .unwrap() from the last round is gone. Now let's stop writing the same match arm three times.

code_arms, from_arms, and strict_from_arms each re-derive (id, lit) from i.wire.as_ref() with an identical let Some(VariantWire::Int(n)) = ... else { unreachable!() } guard — the only difference is the trailing quote!. Every time a field is added to this shape, three near-identical closures need to change in lockstep. Pull the (id, lit) pair once and map each output from it.

♻️ Proposed consolidation

-    let code_arms: Vec<_> = infos
-        .iter()
-        .filter(|i| !i.is_fallback)
-        .map(|i| {
-            let id = &i.ident;
-            let Some(VariantWire::Int(n)) = i.wire.as_ref() else {
-                unreachable!()
-            };
-            let lit = proc_macro2::Literal::i32_suffixed(*n);
-            quote! { `#name`::`#id` => `#lit` }
-        })
-        .collect();
-
-    let from_arms: Vec<_> = infos
-        .iter()
-        .filter(|i| !i.is_fallback)
-        .map(|i| {
-            let id = &i.ident;
-            let Some(VariantWire::Int(n)) = i.wire.as_ref() else {
-                unreachable!()
-            };
-            let lit = proc_macro2::Literal::i32_suffixed(*n);
-            quote! { `#lit` => `#name`::`#id` }
-        })
-        .collect();
-
-    let strict_from_arms: Vec<_> = infos
-        .iter()
-        .filter(|i| !i.is_fallback)
-        .map(|i| {
-            let id = &i.ident;
-            let Some(VariantWire::Int(n)) = i.wire.as_ref() else {
-                unreachable!()
-            };
-            let lit = proc_macro2::Literal::i32_suffixed(*n);
-            quote! { `#lit` => ::core::result::Result::Ok(`#name`::`#id`) }
-        })
-        .collect();
+    let known: Vec<_> = infos
+        .iter()
+        .filter(|i| !i.is_fallback)
+        .map(|i| {
+            let Some(VariantWire::Int(n)) = i.wire.as_ref() else {
+                unreachable!()
+            };
+            (&i.ident, proc_macro2::Literal::i32_suffixed(*n))
+        })
+        .collect();
+    let code_arms: Vec<_> = known.iter().map(|(id, lit)| quote! { `#name`::`#id` => `#lit` }).collect();
+    let from_arms: Vec<_> = known.iter().map(|(id, lit)| quote! { `#lit` => `#name`::`#id` }).collect();
+    let strict_from_arms: Vec<_> = known
+        .iter()
+        .map(|(id, lit)| quote! { `#lit` => ::core::result::Result::Ok(`#name`::`#id`) })
+        .collect();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wacore/derive/src/lib.rs` around lines 1162 - 1199, Consolidate the repeated
`(id, lit)` extraction shared by `code_arms`, `from_arms`, and
`strict_from_arms` by deriving the non-fallback variant data once, including the
existing `VariantWire::Int` validation, then generate each arm collection from
that shared representation. Preserve the three distinct match-arm outputs and
the current unreachable behavior for invalid wire values.